Require tuple before env in the prelude (#8771)#17088
Closed
stakach wants to merge 1 commit into
Closed
Conversation
`{"a", "b"}.as Enumerable(String)` crashed codegen with
`Enumerable::NotFoundError`: `Tuple(String, String)` was missing from
`Enumerable(String)`'s including types.
The cause is require ordering. `Tuple` does `include Indexable(Union(*T))`
(which transitively includes `Enumerable`/`Iterable`), but `tuple` was
required after `env`. `ENV` does `extend Enumerable({String, String})`, which
instantiates `Tuple(String, String)` before `Tuple` gained those module
parents, so its `after_initialize` registered nothing and it never appeared in
`Enumerable(String).including_types`. Codegen then failed to find it when
upcasting. Other arities worked because they were instantiated later.
Requiring `tuple` (and `named_tuple`) in the ordered block right after
`indexable` ensures the includes are processed before any early
instantiation, so all arities register correctly. This replaces the need for
any compiler-side workaround.
A codegen spec covers the original reproducer.
stakach
force-pushed
the
fix-tuple-enumerable-upcast
branch
from
1a665f0 to
a57b0f1
Compare
stakach
changed the title
tuple before env in the prelude (#8771)
Contributor
Author
|
#17094 is a better solution |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #8771 (and its duplicate #17086).
Issue
{"a", "b"}.as Enumerable(String)crashes the compiler:It only happens when a tuple has 2+ elements whose splat-union collapses to a single type (
Union(String, String) == String).{"a"},{"a", 1},{"a", "b", "c"}all work.Root cause
Require ordering.
Tupledoesinclude Indexable(Union(*T))(which transitively includesEnumerable/Iterable), buttuplewas required afterenv.ENVdoesextend Enumerable({String, String}), which instantiatesTuple(String, String)beforeTuplegained those module parents. Itsafter_initializetherefore ran with an incomplete parent list (onlyValue) and never registered the instance as an including type ofEnumerable(String). It was consequently missing fromEnumerable(String).including_types— the union codegen materializes foras Enumerable(String)— soupcast_distinctcouldn't find it and raised. Other arities worked because they're instantiated aftertuple.cris loaded.Fix
Require
tuple(andnamed_tuple) in the prelude's ordered block, right afterindexable, so the tuple includes are processed before any file that can trigger an early tuple instantiation (such asenv). All arities then register correctly. No compiler-side change is needed.A codegen spec covers the original reproducer.